Skip to content

ref(pyreqwest): Move crumbs to integration - #7148

Merged
sentrivana merged 34 commits into
masterfrom
ivana/move-http-crumbs-2
Aug 12, 2026
Merged

ref(pyreqwest): Move crumbs to integration#7148
sentrivana merged 34 commits into
masterfrom
ivana/move-http-crumbs-2

Conversation

@sentrivana

@sentrivana sentrivana commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Description

Create HTTP client breadcrumbs directly in the pyreqwest integration instead of using maybe_create_breadcrumbs_from_span.

Additionally:

  • refactor to use an early return in the wrapper to avoid nesting
  • add breadcrumb tests to the pyreqwest test suite

Issues

Reminders

@github-actions

github-actions Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Codecov Results 📊

111463 passed | ⏭️ 6749 skipped | Total: 118212 | Pass Rate: 94.29% | Execution Time: 404m 41s

📊 Comparison with Base Branch

Metric Change
Total Tests 📈 +108
Passed Tests 📈 +108
Failed Tests
Skipped Tests

All tests are passing successfully.

✅ Patch coverage is 96.55%. Project has 2483 uncovered lines.
✅ Project coverage is 90.2%. Comparing base (base) to head (head).

Files with missing lines (1)
File Patch % Lines
sentry_sdk/integrations/pyreqwest.py 96.43% ⚠️ 1 Missing and 3 partials
Coverage diff
@@            Coverage Diff             @@
##          main       #PR       +/-##
==========================================
+ Coverage    90.16%    90.20%    +0.04%
==========================================
  Files          193       193         —
  Lines        25317     25337       +20
  Branches      9280      9286        +6
==========================================
+ Hits         22827     22854       +27
- Misses        2490      2483        -7
- Partials      1427      1427         —

Generated by Codecov Action

@sentrivana
sentrivana marked this pull request as ready for review August 10, 2026 09:16
@sentrivana
sentrivana requested a review from a team as a code owner August 10, 2026 09:16
Comment thread sentry_sdk/integrations/pyreqwest.py Outdated
Comment thread sentry_sdk/integrations/pyreqwest.py Outdated
Comment thread sentry_sdk/integrations/pyreqwest.py Outdated
@sentrivana
sentrivana marked this pull request as draft August 10, 2026 09:53
@sentrivana
sentrivana marked this pull request as ready for review August 10, 2026 10:21
Comment thread sentry_sdk/integrations/pyreqwest.py

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit e31a3aa. Configure here.

Comment thread tests/integrations/pyreqwest/test_pyreqwest.py
Comment thread sentry_sdk/integrations/pyreqwest.py
Base automatically changed from ivana/move-http-crumbs-1 to master August 12, 2026 11:53
@sentrivana
sentrivana enabled auto-merge (squash) August 12, 2026 11:56
@sentrivana
sentrivana merged commit e4d7398 into master Aug 12, 2026
139 checks passed
@sentrivana
sentrivana deleted the ivana/move-http-crumbs-2 branch August 12, 2026 12:07
from sentry_sdk.traces import StreamedSpan
from sentry_sdk.tracing import BAGGAGE_HEADER_NAME
from sentry_sdk.tracing_utils import (
add_http_breadcrumb,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pyreqwest middleware skips breadcrumbs when HTTP requests raise exceptions

If next_handler.run(request) raises an exception (network error, timeout), the middleware skips breadcrumb creation because it only runs inside if response is not None:. The old fallback via Span.finish() and maybe_create_breadcrumbs_from_span no longer applies since tracing_utils.py now explicitly excludes pyreqwest ("auto.http.pyreqwest") from that path. As a result, connection-level failures produce no HTTP breadcrumb at all. Both sentry_async_middleware (~172) and sentry_sync_middleware (~217) share this flaw.

Evidence
  • sentry_async_middleware (line 172) and sentry_sync_middleware (line 217) set response = None, execute the request inside with _sentry_pyreqwest_span(...), and only call add_http_breadcrumb inside if response is not None: after the block.
  • If next_handler.run(request) raises, control jumps past the crumb code and response stays None.
  • maybe_create_breadcrumbs_from_span in tracing_utils.py:228-246 now skips span.origin == "auto.http.pyreqwest", so no fallback breadcrumb is recorded.
  • StreamedSpan.__exit__ marks the span as error but never creates a breadcrumb, and the old Span.finish() breadcrumb path is likewise blocked for this origin.
  • The new test suite (test_pyreqwest.py) includes happy-path breadcrumb tests but no test simulating a connection-level exception.

Identified by Warden · find-bugs · CGK-YBX

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants